-
Notifications
You must be signed in to change notification settings - Fork 897
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set default OrchestrationTemplateRunner timeout to 100 minutes. #19381
Conversation
@lfu Cannot apply the following label because they are not recognized: service |
@@ -1,5 +1,5 @@ | |||
class ManageIQ::Providers::CloudManager::OrchestrationTemplateRunner < ::Job | |||
DEFAULT_EXECUTION_TTL = 10 # minutes | |||
DEFAULT_EXECUTION_TTL = 100 # minutes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change this constant name to include minutes, perhaps DEFAULT_EXECUTION_TTL_MINUTES
? We generally try to store the base unit of measure: seconds, bytes, etc. I would not expect this to be in minutes.
6317702
to
c5e2dc1
Compare
Checked commit lfu@c5e2dc1 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 app/models/manageiq/providers/cloud_manager/orchestration_template_runner.rb
|
|
||
def minimize_indirect | ||
@minimize_indirect = true if @minimize_indirect.nil? | ||
@minimize_indirect | ||
end | ||
|
||
def current_job_timeout(_timeout_adjustment = 1) | ||
@execution_ttl ||= | ||
(options[:execution_ttl].present? ? options[:execution_ttl].try(:to_i) : DEFAULT_EXECUTION_TTL) * 60 | ||
@execution_ttl ||= options[:execution_ttl].present? ? options[:execution_ttl].to_i.minutes : DEFAULT_EXECUTION_TTL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes a lot more sense to me, thanks @lfu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@lfu Can this be |
@simaishi Yes. |
Set default OrchestrationTemplateRunner timeout to 100 minutes. (cherry picked from commit b61deda) https://bugzilla.redhat.com/show_bug.cgi?id=1756292
Ivanchuk backport details:
|
To match the default automate timeout.
https://bugzilla.redhat.com/show_bug.cgi?id=1756292
@miq-bot assign @tinaafitz
@miq-bot add_label bug, services